-
Notifications
You must be signed in to change notification settings - Fork 286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(daml): creation of connector class #3615
base: main
Are you sure you want to change the base?
feat(daml): creation of connector class #3615
Conversation
886560c
to
cd091b0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please incorporate these changes and then re-request for review (Make sure to squash the commits into a single commit)
"generate-sdk:go": "openapi-generator-cli generate -i ./src/main/json/openapi.json -g go -o ./src/main/go/generated/openapi/go-client/ --git-user-id hyperledger --git-repo-id $(echo $npm_package_name | replace @hyperledger/ \"\" -z)/src/main/go/generated/openapi/go-client --package-name $(echo $npm_package_name | replace @hyperledger/ \"\" -z) --reserved-words-mappings protected=protected --ignore-file-override ../../openapi-generator-ignore", | ||
"generate-sdk:kotlin": "openapi-generator-cli generate -i ./src/main/json/openapi.json -g kotlin -o ./src/main/kotlin/generated/openapi/kotlin-client/ --reserved-words-mappings protected=protected --ignore-file-override ../../openapi-generator-ignore", | ||
"generate-sdk:typescript-axios": "openapi-generator-cli generate -i ./src/main/json/openapi.json -g typescript-axios -o ./src/main/typescript/generated/openapi/typescript-axios/ --ignore-file-override ../../openapi-generator-ignore", | ||
"generate-server": "yarn run --top-level openapi-generator-cli generate -i ./src/main/json/openapi.json -g kotlin-spring -o ./src/main-server/kotlin/gen/kotlin-spring/ -c ./src/main-server/openapi-generator-config.yaml --ignore-file-override ../../openapi-generator-ignore", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not need this. Please remove this (might got have remained when you copied this from corda connector)
} | ||
}, | ||
"operationId": "exerciseChoice", | ||
"summary": "This method creates a simple iou countract", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update the summary and the following description of this path
} | ||
}, | ||
"operationId": "queryRawContract", | ||
"summary": "This method queries DAML contracts", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update this, along with the description and mention why is it different from the above query-iou-endpoint
method
} | ||
}, | ||
"operationId": "getPartiesInvolved", | ||
"summary": "This method queries DAML contracts", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update this to match the referenced path
import type { Server as SecureServer } from "https"; | ||
import type { Config as SshConfig } from "node-ssh"; | ||
import type { Express } from "express"; | ||
// import urlcat from "urlcat"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove all instances of uncommented (unused) code.
try { | ||
if (this.apiUrl === undefined) | ||
throw new InternalServerError("apiUrl option is necessary"); | ||
const body = await this.options.connector.createContract(req.body, "testdata123"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we passing testdata123
here? Should this variable be passed from the test case itself if required
? (And thus be a part of req.body
?)
try { | ||
if (this.apiUrl === undefined) | ||
throw new InternalServerError("apiUrl option is necessary"); | ||
const body = await this.options.connector.exerciseContract(req.body, "testdata123"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we passing testdata123 here? Should this variable be passed from the test case itself if required
? (And thus be a part of req.body?)
} | ||
|
||
async handleRequest(req: Request, res: Response): Promise<void> { | ||
const fnTag = "QueryIOURawEndpointRequest#handleRequest()"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update this to QueryRawIOUEndpoint...
to match the api endpoint
} | ||
const createIou = await apiClient.createIou(iouBody) | ||
console.log("STEP 1. Create IOU as Alice result:") | ||
console.log(createIou) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use LoggerProvider class from cactus-common instead
responseForIOUResult = JSON.parse(stringifyIOUPayload) | ||
|
||
|
||
console.log("STEP 4. Check if transfer is successful by querying as BOB") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rephrase all these statements (along with using LoggerProvider class). Right now it looks like this is the line from where step4 execution starts.
cd091b0
to
20cc75d
Compare
} | ||
|
||
public async transact(): Promise<any> { | ||
return null as any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jagpreetsinghsasan I agree, this looks closer to something application specific instead of belong in the generic framework code. I'm not an expert on DAML/Canton, but maybe we are looking for something like this instead?
https://docs.daml.com/app-dev/grpc/proto-docs.html#submitandwait-method-version-com-daml-ledger-api-v1
1a99027
to
5b9e094
Compare
Hello @petermetz , May I request for you to update the docker image of this task?: Cheers! Rayn |
5b9e094
to
0e0ae2e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raynatopedrajeta Please don't forget to fix the custom-checks that are failing:
[tools/check-missing-node-deps.ts] ERROR - MISSING production dependency node-fetch from /********/packages/cactus-plugin-ledger-connector-daml/package.json. Found usage in
/********/packages/cactus-plugin-ledger-connector-daml/src/main/typescript/plugin-ledger-connector-daml.ts
[tools/check-missing-node-deps.ts] ERROR - MISSING production dependency express from /********/packages/cactus-plugin-ledger-connector-daml/package.json. Found usage in
/********/packages/cactus-plugin-ledger-connector-daml/src/main/typescript/web-services/create-iou-endpoint.ts
/********/packages/cactus-plugin-ledger-connector-daml/src/main/typescript/web-services/exercise-iou-endpoint.ts
/********/packages/cactus-plugin-ledger-connector-daml/src/main/typescript/web-services/get-parties-involved-endpoint.ts
/********/packages/cactus-plugin-ledger-connector-daml/src/main/typescript/web-services/query-iou-endpoint.ts
/********/packages/cactus-plugin-ledger-connector-daml/src/main/typescript/web-services/query-raw-iou-endpoint.ts
"node-fetch" is a missing dependency from /********/packages/cactus-plugin-ledger-connector-daml/package.json with the following files using them:
/********/packages/cactus-plugin-ledger-connector-daml/src/main/typescript/plugin-ledger-connector-daml.ts
0e0ae2e
to
67919b6
Compare
@raynatopedrajeta I tried to publish the image but it fails to upload because one of the layers is 1.5GB and the upload keeps retrying to infinity. Could you please reduce the layer size by splitting the commands in the dockerfile? I'm pretty sure it's this one: # Download and install DAML SDK 2.9.3
RUN curl -L https://github.com/digital-asset/daml/releases/download/v2.9.3/daml-sdk-2.9.3-linux.tar.gz | tar -xz -C /opt && \
cd /opt/sdk-2.9.3 && \
./install.sh && \
rm -rf /opt/sdk-2.9.3/ |
@raynatopedrajeta FYI: I pushed a small change to modify the readme of the DAML aio image so that it's easier to build the image in the future for publishing: ## Build an image locally
To build the daml-all-in-one image locally, use:
```sh
DOCKER_BUILDKIT=1 docker build \
--file ./tools/docker/daml-all-in-one/Dockerfile \
./tools/docker/daml-all-in-one/ \
--tag daio \
--tag daml-all-in-one \
--tag ghcr.io/hyperledger/daml-all-in-one:$(date +"%Y-%m-%dT%H-%M-%S" --utc)-dev-$(git rev-parse --short HEAD)
|
d459b06
to
b000e2a
Compare
@RafaelAPB here's the second PR (the first one got merged sometime back and we had dockerized daml all-in-one image created in that. This PR deals with the creation of DAML connector and test cases which tests the connector with the AIO) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raynatopedrajeta Please see my comments above.
I'll try to publish the container image again in a sec
# # Download and install DAML SDK 2.9.3 | ||
# RUN curl -L https://github.com/digital-asset/daml/releases/download/v2.9.3/daml-sdk-2.9.3-linux.tar.gz | tar -xz -C /opt && \ | ||
# cd /opt/sdk-2.9.3 && \ | ||
# ./install.sh && \ | ||
# rm -rf /opt/sdk-2.9.3/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raynatopedrajeta If we no longer need this commented code then please remove it
const base64UrlEncode = (input: Buffer): string => { | ||
return input | ||
.toString("base64") | ||
.replace(/\+/g, "-") | ||
.replace(/\//g, "_") | ||
.replace(/=+$/, ""); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raynatopedrajeta Please extract this function into a separate code file of its own
const header = base64UrlEncode(Buffer.from('{"alg":"HS256","typ":"JWT"}')); | ||
const payload = base64UrlEncode( | ||
Buffer.from( | ||
`{"https://daml.com/ledger-api": {"ledgerId": "sandbox", "applicationId": "foobar","actAs":["${participant}"]}}`, | ||
), | ||
); | ||
const hmacSignature = base64UrlEncode( | ||
crypto | ||
.createHmac("sha256", "secret") | ||
.update(`${header}.${payload}`) | ||
.digest(), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raynatopedrajeta Please eliminate nested functions calls here (and everywhere in general)
describe("DAML SIMPLE IOU TRANSACTION", () => { | ||
it("DAML SIMPLE IOU TRANSACTION", async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raynatopedrajeta Please do not use caps lock when writing test descriptions, sentence case is OK for example.
// console.log("STEP 1. Create IOU as Alice result:") | ||
// console.log(createIou) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raynatopedrajeta Please remove the commented code if no longer needed
public async hasTransactionFinality(): Promise<boolean> { | ||
const currentConsensusAlgorithmFamily = | ||
await this.getConsensusAlgorithmFamily(); | ||
|
||
return consensusHasTransactionFinality(currentConsensusAlgorithmFamily); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raynatopedrajeta This can be removed because it is no longer part of the connector plugin interface definition IIRC
} | ||
|
||
public async transact(): Promise<any> { | ||
return null as any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raynatopedrajeta Instead of returning null please throw the appropriate HTTP error from the http errors library that we use
My recommendation for the code is this:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/501
} | ||
|
||
public deployContract(): Promise<any> { | ||
throw new Error("Method not implemented."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raynatopedrajeta Instead of throwing a regular error please throw the appropriate HTTP error from the http errors library that we use
My recommendation for the code is this:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/501
...ages/cactus-plugin-ledger-connector-daml/src/main/typescript/plugin-ledger-connector-daml.ts
Outdated
Show resolved
Hide resolved
GetPartiesInvolvedEndpoint, | ||
} from "./web-services/get-parties-involved-endpoint"; | ||
|
||
import fetch from "node-fetch"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raynatopedrajeta Please use axios
instead of node-fetch just to keep the cognitive load minimized for everybody (axios is well used already)
@raynatopedrajeta Unfortunately I still can't publish the image because the problematic (oversized) layer is still 1.5GB and GHCR just won't have it at that size. What I suggest is to refactor the image to have a multi-stage build where you call the daml sdk install in the first stage and then copy over the installed files in 3 or 4 chunks (or whatever makes sense) so that you can control each layer's size that way. I'll paste for reference some of the commands I used while debugging the layer sizes so you can audit it for yourself as well: The install folder that is 1.5GB (needs to be split across layers so that we can upload the image to GHCR) #10 [ 6/17] RUN du -a -x / | sort -n -r | head -n 20
#10 1.070 2233840 /
#10 1.070 1452640 /root
#10 1.070 1452596 /root/.daml
#10 1.070 1452572 /root/.daml/sdk
#10 1.070 1452568 /root/.daml/sdk/2.9.3
#10 1.070 788968 /root/.daml/sdk/2.9.3/damlc
#10 1.070 709976 /usr
#10 1.070 603948 /usr/lib
#10 1.070 472316 /root/.daml/sdk/2.9.3/damlc/lib
#10 1.070 316644 /root/.daml/sdk/2.9.3/damlc/resources
#10 1.070 316644 /root/.daml/sdk/2.9.3/damlc/lib/resources
#10 1.070 292452 /usr/lib/jvm
#10 1.070 292440 /usr/lib/jvm/java-21-openjdk-amd64
#10 1.070 273036 /usr/lib/x86_64-linux-gnu
#10 1.070 225988 /root/.daml/sdk/2.9.3/canton
#10 1.070 225984 /root/.daml/sdk/2.9.3/canton/canton.jar
#10 1.070 207988 /usr/lib/jvm/java-21-openjdk-amd64/lib
#10 1.070 155492 /root/.daml/sdk/2.9.3/daml-sdk
#10 1.070 155456 /root/.daml/sdk/2.9.3/daml-sdk/daml-sdk.jar
#10 1.070 151100 /root/.daml/sdk/2.9.3/damlc/lib/damlc
#10 DONE 1.4s Build command with DOCKER_BUILDKIT=1 docker build --progress=plain --file ./tools/docker/daml-all-in-one/Dockerfile ./tools/docker/daml-all-in-one/ --tag daio --tag daml-all-in-one --tag ghcr.io/hyperledger/daml-all-in-one:$(date +"%Y-%m-%dT%H-%M-%S" --utc)-dev-$(git rev-parse --short HEAD) An example of how to list the layers of any image with their sizes included (all layers must be below 500 MB otherwise we won't be able to reliably upload/pull them to/from GHCR) #22 exporting to image
#22 exporting layers done
#22 writing image sha256:1b518c9931faccb21acba11e72827b74016516b9a5167693ccabf2e6ed32e408 done
#22 naming to docker.io/library/daio done
#22 naming to docker.io/library/daml-all-in-one done
#22 naming to ghcr.io/hyperledger/daml-all-in-one:2024-11-21T16-31-25-dev-b000e2a6a done
#22 DONE 0.0s
1 warning found (use docker --debug to expand):
- LegacyKeyValueFormat: "LABEL key=value" should be used instead of legacy "LABEL key value" format (line 3)
peter@zend:~/a/cacti-upstream$ docker history --no-trunc 1b518c9931faccb21acba11e72827b74016516b9a5167693ccabf2e6ed32e408
IMAGE CREATED CREATED BY SIZE COMMENT
sha256:1b518c9931faccb21acba11e72827b74016516b9a5167693ccabf2e6ed32e408 About an hour ago HEALTHCHECK &{["CMD-SHELL" "/quickstart/healthcheck.sh"] "30s" "1m0s" "1m40s" "0s" 'd'} 0B buildkit.dockerfile.v0
<missing> About an hour ago RUN /bin/sh -c chmod +x /quickstart/healthcheck.sh # buildkit 190B buildkit.dockerfile.v0
<missing> About an hour ago COPY healthcheck.sh /quickstart/healthcheck.sh # buildkit 190B buildkit.dockerfile.v0
<missing> About an hour ago CMD ["--configuration" "/etc/supervisord.conf" "--nodaemon"] 0B buildkit.dockerfile.v0
<missing> About an hour ago ENTRYPOINT ["/usr/bin/supervisord"] 0B buildkit.dockerfile.v0
<missing> About an hour ago EXPOSE map[7575/tcp:{}] 0B buildkit.dockerfile.v0
<missing> About an hour ago EXPOSE map[9001/tcp:{}] 0B buildkit.dockerfile.v0
<missing> About an hour ago COPY supervisord.conf /etc/supervisord.conf # buildkit 1.45kB buildkit.dockerfile.v0
<missing> About an hour ago RUN /bin/sh -c mkdir -p /var/log/supervisor # buildkit 0B buildkit.dockerfile.v0
<missing> About an hour ago RUN /bin/sh -c /quickstart/generate-jwt-token.sh # buildkit 217B buildkit.dockerfile.v0
<missing> About an hour ago RUN /bin/sh -c chmod +x /quickstart/generate-jwt-token.sh # buildkit 597B buildkit.dockerfile.v0
<missing> About an hour ago COPY generate-jwt-token.sh /quickstart/generate-jwt-token.sh # buildkit 597B buildkit.dockerfile.v0
<missing> About an hour ago RUN /bin/sh -c echo '{"server": {"address": "0.0.0.0","port": 7575},"ledger-api": {"address": "0.0.0.0","port": 6865}}' > json-api-app.conf # buildkit 98B buildkit.dockerfile.v0
<missing> About an hour ago WORKDIR /quickstart 0B buildkit.dockerfile.v0
<missing> About an hour ago RUN /bin/sh -c daml new quickstart --template quickstart-java # buildkit 32kB buildkit.dockerfile.v0
<missing> About an hour ago ENV PATH=/root/.daml/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin 0B buildkit.dockerfile.v0
<missing> About an hour ago RUN /bin/sh -c rm -rf /opt/sdk-2.9.3 /tmp/daml-sdk-2.9.3-linux.tar.gz # buildkit 0B buildkit.dockerfile.v0
<missing> About an hour ago RUN /bin/sh -c cd /opt/sdk-2.9.3 && ./install.sh # buildkit 1.48GB buildkit.dockerfile.v0
<missing> About an hour ago RUN /bin/sh -c tar -xz -C /opt -f /tmp/daml-sdk-2.9.3-linux.tar.gz # buildkit 1.16GB buildkit.dockerfile.v0
<missing> About an hour ago RUN /bin/sh -c curl -L https://github.com/digital-asset/daml/releases/download/v2.9.3/daml-sdk-2.9.3-linux.tar.gz -o /tmp/daml-sdk-2.9.3-linux.tar.gz # buildkit 689MB buildkit.dockerfile.v0
<missing> 6 days ago RUN /bin/sh -c apt update && apt install --no-install-recommends curl openjdk-21-jdk supervisor openssl xxd -y # buildkit 676MB buildkit.dockerfile.v0
<missing> 6 days ago LABEL org.opencontainers.image.source== https://github.com/hyperledger/cacti 0B buildkit.dockerfile.v0
<missing> 2 months ago /bin/sh -c #(nop) CMD ["/bin/bash"] 0B
<missing> 2 months ago /bin/sh -c #(nop) ADD file:ebe009f86035c175ba244badd298a2582914415cf62783d510eab3a311a5d4e1 in / 77.9MB
<missing> 2 months ago /bin/sh -c #(nop) LABEL org.opencontainers.image.version=22.04 0B
<missing> 2 months ago /bin/sh -c #(nop) LABEL org.opencontainers.image.ref.name=ubuntu 0B
<missing> 2 months ago /bin/sh -c #(nop) ARG LAUNCHPAD_BUILD_ARCH 0B
<missing> 2 months ago /bin/sh -c #(nop) ARG RELEASE 0B Notice this layer is the problematic one from the above excerpt: RUN /bin/sh -c cd /opt/sdk-2.9.3 && ./install.sh # buildkit 1.48GB |
Primary Changes --------------- 1. Create a DAML connector class 2. Created OpenAPI endpoints of DAML 3. Created DAML web services for create, exercise and query contracts 4. Create simple IOU Transaction using DAML Fixes hyperledger-cacti#3489 Co-authored-by: Peter Somogyvari <[email protected]> Signed-off-by: raynato.c.pedrajeta <[email protected]> Signed-off-by: Peter Somogyvari <[email protected]>
b000e2a
to
12c65f9
Compare
Hello @petermetz , On this part, I managed to trim down the memory used on all of the layers of the Dockerfile. I also implemented multi-stage build of docker upon doing so: However, on the installation part of daml(install.sh), I wasn't still able to trim down the memory used of the layer. Please let me know what will be the next plans for this. Thank you! Rayn |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raynatopedrajeta Please move the install.sh script execution into one of the layers (as in, build stages that are not in the last stage). Nothing else needs to be in separate stages, just the problematic operation that generates the huge layer that cannot be pushed.
Commit to be reviewed
feat(daml): creation of connector class
Fixes #3489